Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React 19 #3398

Merged
merged 6 commits into from
Jan 16, 2025
Merged

React 19 #3398

merged 6 commits into from
Jan 16, 2025

Conversation

vicgeralds
Copy link
Contributor

@vicgeralds vicgeralds commented Jan 14, 2025

  • Updates to React 19 which comes with better support for rendering custom elements
  • Removes LimeElementsAdapter that's used for rendering custom elements in rjsf forms

This is a fix, because LimeElementsAdapter set some properties later depending on their type, meaning they were set later than expected in some situations or in unexpected order.

Review:

  • Commits are atomic
  • Commits have the correct type for the changes made
  • Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

  • Chrome
  • Edge
  • Firefox

Linux:

  • Chrome
  • Firefox

macOS:

  • Chrome
  • Firefox
  • Safari

Mobile:

  • Chrome on Android
  • iOS

Copy link

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3398/

@vicgeralds vicgeralds marked this pull request as ready for review January 14, 2025 10:08
@vicgeralds vicgeralds requested a review from a team as a code owner January 14, 2025 10:08
@vicgeralds vicgeralds force-pushed the react-19 branch 3 times, most recently from 19dc4cf to 4d3328b Compare January 14, 2025 13:50
It's no longer needed when using React 19 which has much better support
for rendering custom elements.

This is a fix, because LimeElementsAdapter set some properties later
depending on their type, meaning they were set later than expected in
some situations or in unexpected order.
Comment on lines -12 to +11
events?: { [key: string]: (event: any) => void };
events: { change: (event: CustomEvent) => void };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm… this changes the object from allowing any number of properties ([key: string]), to only allowing a single property called event. Was that really intentional?

How about the following?

    events: {
        change: (event: CustomEvent) => void,
        [key: string]: (event: any) => void,
    };

This makes the object required, and it has to have the change property, but it can also have any number of extra properties.

Although, come to think of it, this commit doesn't change lime-elements.api.md, so I guess this is a purely internal change anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was that really intentional?

Yes. That’s how it’s actually used, and if we need to handle more events, we can add them. It’s clearer what events need to be handled and easier to implement than allowing any event.

so I guess this is a purely internal change anyway?

Correct

adrianschmidt
adrianschmidt previously approved these changes Jan 16, 2025
Copy link
Contributor

@adrianschmidt adrianschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work flawlessly… 😅

Nice work! 💪

@adrianschmidt adrianschmidt enabled auto-merge (rebase) January 16, 2025 16:55
@adrianschmidt adrianschmidt enabled auto-merge (rebase) January 16, 2025 16:56
@adrianschmidt adrianschmidt dismissed their stale review January 16, 2025 16:58

I just want to check one thing :)

@adrianschmidt adrianschmidt merged commit b7f5264 into main Jan 16, 2025
12 checks passed
@adrianschmidt adrianschmidt deleted the react-19 branch January 16, 2025 18:57
@lime-opensource
Copy link
Collaborator

🎉 This PR is included in version 37.79.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants